Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Master.crtp position update #147

Merged
merged 17 commits into from
Sep 22, 2016
Merged

Master.crtp position update #147

merged 17 commits into from
Sep 22, 2016

Conversation

stephanbro
Copy link
Contributor

This pull request adds a new port to CRTP for sending an external position. The commander registers the callback and buffers the data, while the stabilizer calls the commander to queue it's current data.

@carlin-psvl
Copy link
Contributor

To clarify the purpose of these commits, it allows the CF2 to be controlled by a motion capture or similar system when the EKF estimate fails or the CF2 starts to move out of LPS range. This is useful in testing or demonstration, as you can provide a virtual safety net of sorts.

@ataffanel
Copy link
Member

Thanks, this looks good. I only have one comment: I would like this functionality to be put in another file than the commander. Commander.c is already too big and full of random stuff. What about moving the code to extPosition.c or any other name that makes sense?

I like the renaming of the commander port in setpoint port. It makes more sense to call it setpoint.

@tobbeanton
Copy link
Member

Shouldn't we use a new "channel" in the commander/setpoint port for the position setpoint rather then occupying a new port?

@ataffanel
Copy link
Member

No I do not think it should be on the same port: the commander is about setpoint, this is more like an external sensor. To me it makes sense to have them separated.

@tobbeanton
Copy link
Member

I see, the commender is about commanding things and the position is just information. Agreed we should keep them separated.

@stephanbro
Copy link
Contributor Author

Thanks for the feedback! I've moved the external position items to their own file. Let me know if you have any other changes you'd like me to make.

I also noticed that libdw1000's submodule wasn't pointing at master as of d38113b and doesn't actually build with -DDWM1000_ENABLE_TDMA! I added a commit to point it back to master. It might be good to add that build config to TravisCI.

@ataffanel ataffanel merged commit 1424bf8 into bitcraze:master Sep 22, 2016
@ataffanel
Copy link
Member

I merged it. I am not so sure about where to call getExtPosition but this is now together with the kalman code that we intend to clean-up anyway (it would be nice to keep the stabilizer loop generic).

Thanks! it is nice to have this in the master banch.

@stephanbro stephanbro deleted the master.crtp_position_update branch October 3, 2016 16:36
@krichardsson krichardsson added this to the Next version milestone Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants